bugfix(ai): Fix AICommandParmsStorage::doXfer to transfer the command source - #3156

• Fix AICommandParmsStorage::doXfer to serialize m_cmdSource instead of duplicating m_cmd.
• Prevent pending AI commands from executing with an uninitialized command source after save/load.
• Apply the fix consistently to both Generals and Zero Hour codepaths.

graph TD
  XFER["Xfer (save/load)"] --> STORAGE["AICommandParmsStorage::doXfer"] --> CMD["m_cmd"] --> AIUPD["AIUpdate pending cmd"] --> EXEC["AI executes command"]
  STORAGE --> CMDSRC["m_cmdSource"] --> AIUPD

Loading

The following are alternative approaches to this PR:

1. Gate behavior behind an Xfer version bump

• ➕ Old saves could default cmdSource instead of inheriting duplicated command bytes
• ➕ Provides explicit long-term control over save compatibility semantics
• ➖ Requires versioning changes at the caller/containing record(s)
• ➖ Slightly more invasive and needs broader testing across versions

2. Add a constructor/default initialization for AICommandParmsStorage

• ➕ Eliminates uninitialized-field risk even if future serialization regressions occur
• ➕ Improves defensive robustness for any non-xfer initialization paths
• ➖ Does not fix the missing serialization by itself (still need this PR’s change)
• ➖ Potentially masks serialization bugs by always supplying a default

Recommendation: Keep the current minimal fix: it corrects the field transfer without changing record layout, and it makes loading older saves deterministic (bounded enum value rather than uninitialized memory). If maintaining an exact default cmdSource for pre-fix saves is a requirement, add a version bump/gate at the containing xfer layer.

Bug fix (2) +4 / -2

AIStates.cppFix doXfer to serialize m_cmdSource (Generals) +2/-1

Fix doXfer to serialize m_cmdSource (Generals)

• Corrects AICommandParmsStorage::doXfer to transfer m_cmdSource instead of transferring m_cmd twice. Prevents pending AI commands from using an uninitialized command source after loading a save.

Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp

AIStates.cppFix doXfer to serialize m_cmdSource (Zero Hour) +2/-1

Fix doXfer to serialize m_cmdSource (Zero Hour)

• Mirrors the Generals fix in the Zero Hour codebase by transferring m_cmdSource during AICommandParmsStorage::doXfer. Ensures consistent save/load behavior across both games.

GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp

Description

When loading a save created before this fix, the slot now read into m_cmdSource contains the old
duplicated m_cmd bytes, producing out-of-range CommandSourceType values. That invalid value is
later used unchecked (e.g., (1 << cmdSource) in weapon selection), which can cause undefined
behavior/crashes and incorrect command-source filtering.

Code

Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp[R129-130]

+	// TheSuperHackers @bugfix Transfer the command source instead of the command twice.
+	xfer->xferUser(&m_cmdSource, sizeof(m_cmdSource));

Evidence

The PR change causes the second serialized field to be interpreted as CommandSourceType on load;
for older saves that field contains duplicated AICommandType bytes. CommandSourceType is a small
enum, but downstream code uses it as a shift count (1 << cmdSource), which becomes undefined
behavior for out-of-range values, so the loaded value must be validated/clamped.

Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp[126-134]
Core/GameEngine/Include/Common/GameCommon.h[217-227]
Generals/Code/GameEngine/Include/GameLogic/AI.h[345-365]
Generals/Code/GameEngine/Source/GameLogic/Object/WeaponSet.cpp[812-820]
Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DeployStyleAIUpdate.cpp[478-513]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
After this PR, `AICommandParmsStorage::doXfer` correctly transfers `m_cmdSource`, but **old saves written by the previous code** stored a duplicate of `m_cmd` in the second field. Loading those saves will therefore set `m_cmdSource` to an invalid `CommandSourceType` value.

This becomes dangerous because other code paths assume `CommandSourceType` is in-range and use it in bit operations (e.g., `1 << cmdSource`), which can become undefined behavior if `cmdSource` is large.

### Issue Context
- `CommandSourceType` has only a few valid values (`CMD_FROM_PLAYER`..`CMD_DEFAULT_SWITCH_WEAPON`, then `COMMAND_SOURCE_TYPE_COUNT`).
- `AICommandType` has many values and can exceed the bit-width safe range for shifting.
- `AICommandParmsStorage::doXfer` currently does not validate `m_cmdSource` after loading.

### Fix Focus Areas
- Add a post-load validation step in `AICommandParmsStorage::doXfer` to clamp/normalize `m_cmdSource` when `xfer->getXferMode() == XFER_LOAD`.
 - Example approach: after `xferUser(&m_cmdSource, ...)`, check `static_cast<Int>(m_cmdSource)` is within `[0, COMMAND_SOURCE_TYPE_COUNT)`; if not, set to a safe default (likely `CMD_FROM_AI`).
- Apply the same fix in both game variants.

Recommended code shape (illustrative):
```cpp
xfer->xferUser(&m_cmdSource, sizeof(m_cmdSource));
if (xfer->getXferMode() == XFER_LOAD) {
 const Int cs = static_cast<Int>(m_cmdSource);
 if (cs < 0 || cs >= COMMAND_SOURCE_TYPE_COUNT) {
   m_cmdSource = CMD_FROM_AI;
 }
}
```

### Fix Focus Areas (exact locations)
- Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp[126-133]
- GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp[129-136]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗